New braille IPC APIs: BrailleMirror, DirectBrailleWindow, and named-pipe server#19856
Draft
trypsynth wants to merge 6 commits intonvaccess:masterfrom
Draft
New braille IPC APIs: BrailleMirror, DirectBrailleWindow, and named-pipe server#19856trypsynth wants to merge 6 commits intonvaccess:masterfrom
trypsynth wants to merge 6 commits intonvaccess:masterfrom
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Closes #19843
Summary of the issue:
External processes (e.g. RIM's elevated service component) have no supported way to receive a live mirror of NVDA's braille output or to act as a direct braille display for a foreground window. The existing
pre_writeCells/filter_displayDimensions/decide_enabledextension points are not sufficient on their own: they require in-process code, have no HWND-scoping, and block the NVDA main thread if pipe I/O is done naively.Description of user facing changes:
None for end users. NVDA Remote users with braille displays can continue using braille normally; the refactor is transparent.
Description of developer facing changes:
Two new public APIs in
braille:BrailleMirror- abstract class. Register withbraille.registerMirror(); receives everydisplay()call and optionally caps the display width vianumCells(). Unregister withbraille.unregisterMirror().DirectBrailleWindow- HWND-scoped class. While its window is foreground, NVDA rendering is suspended (decide_enabled) and braille gestures are intercepted (decide_executeGesture) and forwarded toonGesture(). Subclass and callactivate()/deactivate().braille.injectGesture(gesture)- helper to execute aBrailleDisplayGestureviainputCore.manager, swallowingNoInputGestureAction.New module
braillePipeServerexposes both APIs over a named pipe (\.\pipe\nvda_braille/\.\pipe\nvda_braille_secure). Wire protocol: 4-byte LE uint32 length-prefix + UTF-8 JSON. Pipe DACL grantsNT AUTHORITY\SYSTEMread/write so RIM's elevated service can connect on the normal desktop.Description of development approach:
registerMirrorlazily hookspre_writeCellsandfilter_displayDimensions(shared single handler); unhooks when last mirror unregisters.DirectBrailleWindowhooksdecide_enabledandinputCore.decide_executeGestureper instance; optionally hooksfilter_displayDimensionsifnumCells > 0.braillePipeServeruses one accept-loop thread + one_AsyncWriterbackground thread per connection so pipe I/O never touches the NVDA main thread._remoteClientrefactored:FollowerSessionnow uses_FollowerBrailleMirrorinstead of directpre_writeCellswiring;localMachine.setBrailleDisplaySize/_handleFilterDisplayDimensionsremoved; pre-existingfilter_displayDimensionsleak on transport close fixed.Testing strategy:
30 unit tests added in
tests/unit/test_braille/test_mirrorAndDirectWindow.pycovering:BrailleMirrorregister/unregister lifecycle and extension-point hook/unhooknumCellscapping logic (zero, smaller, larger, smallest-wins)injectGesturehappy path andNoInputGestureActionswallowDirectBrailleWindowactivate/deactivate idempotency,decide_enabledanddecide_executeGesturerouting,filter_displayDimensionsregistration and cappingManual testing needed: NVDA Remote with a physical braille display on both sides. I've locally tested with a 20 cell on both sides individually, but am unable to test both at the same time
Known issues with pull request:
RIM client-side integration is not yet complete. Do not merge until the RIM release is coordinated.
LeaderSessionin NVDA Remote still wiresdecide_executeGesturedirectly rather than viaDirectBrailleWindow; should this be migrated in core as well? Needs discussion.Code Review Checklist: